Skip to content

Allow additional settings for GSSAPI in Vhost #2215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 18, 2022

Conversation

tuxmea
Copy link
Contributor

@tuxmea tuxmea commented Feb 18, 2022

Adding config items:

  • allowedmech
  • authname
  • authtype
  • basicauth

bastelfreak
bastelfreak previously approved these changes Mar 18, 2022
@david22swan
Copy link
Member

@tuxmea Sorry for the wait on review. Anyway this change look's good to me but could you also update the relevant documentation linked below? Would be sad if you put in a great change like this and nobody could find it:

@david22swan
Copy link
Member

@tuxmea Sorry to come back but while reviewing other prs I noticed that a similar change had been pushed up by another contributor that contains part of the functionality that you yourself have added. Just felt I should alert you as depending on which PR is merged first you may need to rebase your work.
Have placed a similar comment on the other PR, linked below:

@david22swan
Copy link
Member

In relation to the comment above, have discovered two more PRs that are adding similar functionality to this, one of which was in a good enough state to be merged:

And another which will require some more work:

@david22swan
Copy link
Member

@tuxmea Hey, just giving an update but on of the PRs I previously mentioned has been merged in so that now allowedmech and basicauth are already included as options.
If you could rebase your code and add the documentation I had previously mentioned however I would be happy to merge this in and get the remaining two options you have tried to add into the codebase.

Copy link
Collaborator

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is now a merge conflict. While some more parameters have been added, at least AuthName and AuthType have not so there's still value in this PR. Please rebase to resolve the conflicts.

Adding config items:
- allowedmech
- authname
- authtype
- basicauth
@tuxmea tuxmea force-pushed the allow_gssapi_krb5_config branch from 5f0787f to 75f6854 Compare July 15, 2022 13:04
@tuxmea
Copy link
Contributor Author

tuxmea commented Jul 15, 2022

@david22swan @ekohl many thanks for your feedback. PR is rebased and ready to get merged. I also updated the documentation in manifests/vhost.pp

@tuxmea
Copy link
Contributor Author

tuxmea commented Jul 15, 2022

Failed test for OracleLinux 6:

Setting up Install Process

stderr:rpmdb: unable to join the environment
error: db3 error(11) from dbenv->open: Resource temporarily unavailable
error: cannot open Packages index using db3 - Resource temporarily unavailable (11)
error: cannot open Packages database in /var/lib/rp

Copy link
Collaborator

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec failure looks odd. I'm not sure how this PR could have caused that and we also see it in nightly: https://github.com/puppetlabs/puppetlabs-apache/runs/7381259870?check_suite_focus=true (that said, it is still odd that in Puppet 6 it fails, but doesn't in 7).

@ekohl ekohl merged commit 943dc9f into puppetlabs:main Jul 18, 2022
@tuxmea tuxmea deleted the allow_gssapi_krb5_config branch July 18, 2022 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants